Skip to content

Add ability to run on Apple silicon #25

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Dec 14, 2021
Merged

Add ability to run on Apple silicon #25

merged 3 commits into from
Dec 14, 2021

Conversation

pauldorehill
Copy link
Contributor

Hi,

This allows the use of the plugin on macOS with Apple silicon and addresses #24. While the issue has nothing to do with this package (the cause is the wasm-pack npm package will throw on macOS unless arch === "x64"), it provides a work around where the user can use their local installation of wasm-pack.

One thing to also note is the following exception that throws Error: no prebuilt wasm-opt binaries are available for this platform: Unrecognized target!. However this can also be fixed by adding the following in the Cargo.toml:

[package.metadata.wasm-pack.profile.release]
wasm-opt = false

[package.metadata.wasm-pack.profile.dev]
wasm-opt = false

@pauldorehill pauldorehill changed the title Add abilty to run on Apple silicon Add ability to run on Apple silicon Dec 7, 2021
@Pauan
Copy link
Collaborator

Pauan commented Dec 7, 2021

I would rather provide an option to specify a custom path to wasm-pack, instead of trying to auto-detect things.

Also, note that optionalDependencies does not really make the dependency optional, it will still be installed regardless.

@pauldorehill
Copy link
Contributor Author

I agree that would be better 😄 - I'll take a look at how to do it.

This is an easy fix in the wasm-pack package... however to get that updated is probably unlikely.

The reason for moving it to an optional dependency is that npm install will fail for all packages and not install anything on Apple silicon: the wasm-pack code runs a postinstall script that throws. Making it optional allows all the other packages to get installed.

@pauldorehill
Copy link
Contributor Author

Ok.. so I've updated to add a wasmPackPath option.

I actually realized it that it will run ok so long as wasm-pack is installed globally... so if you wanted to keep it simple all that needs to be changed for it to work is the optionalDependancy?

Pauan
Pauan requested changes Dec 12, 2021
View reviewed changes
Copy link
Collaborator

@Pauan Pauan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking a lot better, just a few minor things to fix.

@pauldorehill
Copy link
Contributor Author

ok should be addressed.

@Pauan Pauan merged commit c3e0676 into wasm-tool:master Dec 14, 2021
@Pauan
Copy link
Collaborator

Pauan commented Dec 14, 2021

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Pauan Pauan Pauan requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pauldorehill @Pauan